Skip to content

Update to follow draft-14#263

Closed
tobbee wants to merge 4 commits intomengelbart:mainfrom
Eyevinn:feat/draft-14-update
Closed

Update to follow draft-14#263
tobbee wants to merge 4 commits intomengelbart:mainfrom
Eyevinn:feat/draft-14-update

Conversation

@tobbee
Copy link
Copy Markdown
Contributor

@tobbee tobbee commented Jan 13, 2026

This PR should be a complete update to draft-14 syntax including renaming, changes of protocol version, and checking the double requestIDs in SUBSCRIBE_UPDATE.

Compared to #254, the renaming should be more complete. Here is a Claude analysis of the differences:

Both commits implement draft-14 message naming updates, but commit 23906d8 (your fork) has several important additions and fixes compared to commit 31744d4 (upstream):

Key Differences

  1. Track Status Message Split ⭐ (Most significant change)

Your commit (23906d8):

  • Splits track status into TWO separate messages:
    • TRACK_STATUS (0x0d): Request message with TrackNamespace + TrackName
    • TRACK_STATUS_OK (0x0e): Response message with StatusCode + LargestLocation
  • Creates new files: track_status_ok_message.go and corresponding tests
  • Deletes: track_status_request_message.go

Upstream commit (31744d4):

  • Only has TRACK_STATUS message (doesn't split into request/response)
  • Keeps the old structure

This is a breaking protocol change that better aligns with draft-14's request/response pattern.

  1. SUBSCRIBE_UPDATE Enhancement ⭐

Your commit (23906d8):

  • Adds SubscriptionRequestID field to track the original SUBSCRIBE request
  • Generates new RequestID for each UPDATE message
  • Updates UpdateSubscription() API to use subscriptionRequestID parameter

Upstream commit (31744d4):

  • Only has basic SUBSCRIBE_UPDATE changes
  • Doesn't include the SubscriptionRequestID field
  1. Bug Fixes

Your commit (23906d8):

  • Fixes typo: ObjectForwardingPreferenceDatagarm → ObjectForwardingPreferenceDatagram (object.go:7)
  • Fixes typo: PUBLISH_NAMESPACE_DONE message mapping (was UNANNOUNCE)

Upstream commit (31744d4):

  • Contains the typo in publish_error_message.go
  1. Code Quality Improvements

Your commit:

  • Better logging in subscribe_update_message.go (proper uint64 casting for slog)
  • More comprehensive test coverage for SUBSCRIBE_UPDATE with SubscriptionRequestID
  • Cleaner session.go handler method names (e.g., onPublishNamespace vs onAnnounce)
  1. Session Handler Updates

Both commits rename the handler methods from:

  • onAnnounce → onPublishNamespace
  • onSubscribeAnnounces → onSubscribeNamespace
  • etc.

But your commit has more complete implementation in session.go with proper RequestID handling.

- Rename SUBSCRIBE_DONE to PUBLISH_DONE (0x0b)
- Rename ANNOUNCE family to PUBLISH_NAMESPACE family:
  - ANNOUNCE -> PUBLISH_NAMESPACE (0x06)
  - ANNOUNCE_OK -> PUBLISH_NAMESPACE_OK (0x07)
  - ANNOUNCE_ERROR -> PUBLISH_NAMESPACE_ERROR (0x08)
  - UNANNOUNCE -> PUBLISH_NAMESPACE_DONE (0x09)
  - ANNOUNCE_CANCEL -> PUBLISH_NAMESPACE_CANCEL (0x0c)
- Rename SUBSCRIBE_ANNOUNCES family to SUBSCRIBE_NAMESPACE family:
  - SUBSCRIBE_ANNOUNCES -> SUBSCRIBE_NAMESPACE (0x11)
  - SUBSCRIBE_ANNOUNCES_OK -> SUBSCRIBE_NAMESPACE_OK (0x12)
  - SUBSCRIBE_ANNOUNCES_ERROR -> SUBSCRIBE_NAMESPACE_ERROR (0x13)
  - UNSUBSCRIBE_ANNOUNCES -> UNSUBSCRIBE_NAMESPACE (0x14)
- Rename TRACK_STATUS messages for consistency:
  - TRACK_STATUS_REQUEST -> TRACK_STATUS (0x0d)
  - TRACK_STATUS -> TRACK_STATUS_OK (0x0e)
- Add SubscriptionRequestID field to SUBSCRIBE_UPDATE message
Per draft-14, SUBSCRIBE_UPDATE increments the request ID counter and
is subject to flow control. Add validation of the new Request ID
against localMaxRequestID to enforce this.
@tobbee tobbee force-pushed the feat/draft-14-update branch from 300af47 to c6acce0 Compare January 13, 2026 14:56
@tobbee tobbee closed this Apr 8, 2026
@tobbee tobbee deleted the feat/draft-14-update branch April 8, 2026 20:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant